-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Semigroup (Predicate a) instance #39
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM besides one quibble about CPP usage.
@@ -310,9 +310,18 @@ newtype Predicate a = Predicate { getPredicate :: a -> Bool } | |||
instance Contravariant Predicate where | |||
contramap f g = Predicate $ getPredicate g . f | |||
|
|||
#if defined(MIN_VERSION_semigroups) || __GLASGOW_HASKELL__ >= 711 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use MIN_VERSION_base(4,9,0)
instead of __GLASGOW_HASKELL__ >= 711
.
instance Monoid (Predicate a) where | ||
mempty = Predicate $ const True | ||
#if defined(MIN_VERSION_semigroups) || __GLASGOW_HASKELL__ >= 711 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
I copied the CPP conditional from other parts of the file, should I change them all into MIN_VERSION_base? Probably `tagged`&`Proxy` too?
…Sent from my iPhone
On 7 Jan 2018, at 22.40, Ryan Scott ***@***.***> wrote:
@RyanGlScott requested changes on this pull request.
LGTM besides one quibble about CPP usage.
In src/Data/Functor/Contravariant.hs:
> @@ -310,9 +310,18 @@ newtype Predicate a = Predicate { getPredicate :: a -> Bool }
instance Contravariant Predicate where
contramap f g = Predicate $ getPredicate g . f
+#if defined(MIN_VERSION_semigroups) || __GLASGOW_HASKELL__ >= 711
Use MIN_VERSION_base(4,9,0) instead of __GLASGOW_HASKELL__ >= 711.
In src/Data/Functor/Contravariant.hs:
> instance Monoid (Predicate a) where
mempty = Predicate $ const True
+#if defined(MIN_VERSION_semigroups) || __GLASGOW_HASKELL__ >= 711
Same here.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Ah, fair enough. I (personally) believe we should eschew this use of |
Yes, let's use Let's merge this, and do refactoring later. |
Fixes #38